Skip to content

openssl_legacy: init and use#202126

Merged
mweinelt merged 5 commits intoNixOS:stagingfrom
helsinki-systems:init/openssl_legacy
Nov 26, 2022
Merged

openssl_legacy: init and use#202126
mweinelt merged 5 commits intoNixOS:stagingfrom
helsinki-systems:init/openssl_legacy

Conversation

@ajs124
Copy link
Member

@ajs124 ajs124 commented Nov 21, 2022

Description of changes

Follow-up to #196906

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ajs124 ajs124 requested review from mweinelt and vcunat November 21, 2022 02:06
@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Nov 21, 2022
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 8.has: package (new) This PR adds a new package labels Nov 21, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Nov 21, 2022
@vcunat
Copy link
Member

vcunat commented Nov 21, 2022

I don't think we can catch this amount of rebuilds before branch-off, unless @mweinelt is OK with delaying it by a few days. (*-darwin stdenvs get rebuilt) Of course, that still doesn't prevent it from getting staged for both 21.11 and unstable.

@vcunat
Copy link
Member

vcunat commented Nov 21, 2022 •
edited
Loading

python310Packages.ntlm-auth builds with this, at least. (on x86_64-linux)

@vcunat vcunat mentioned this pull request Nov 21, 2022
Merged
13 tasks
@mweinelt
Copy link
Member

mweinelt commented Nov 21, 2022

I think at this point we should go with staging-22.11.

@ajs124
Copy link
Member Author

ajs124 commented Nov 21, 2022

Alright, so I'll retarget this against staging and we backport it into staging-22.11, once that's created?

@ajs124
openssl_legacy: init
1996190 
openssl_3, but with a openssl.cnf that enables legacy ciphers
this way we can migrate away from openssl_1_1, while not breaking
applications relying on deprecated stuff
@ajs124
Revert "Revert "python3: pin to openssl_1_1""
f06f950 
This reverts commit 017fd89.
@ajs124
python3: use openssl_legacy
2b3c729
@ajs124
Revert "openldap: disable failing test"
578b6d3 
This reverts commit 66e6f6f.
@ajs124
openldap: use openssl_legacy
ff4dcc5
@ajs124 ajs124 force-pushed the init/openssl_legacy branch from aa36b1b to ff4dcc5 Compare November 21, 2022 12:46
@ajs124 ajs124 changed the base branch from staging-next to staging November 21, 2022 12:46
@ajs124 ajs124 marked this pull request as ready for review November 21, 2022 12:46
@ajs124 ajs124 mentioned this pull request Nov 21, 2022
Merged
13 tasks
@dasJ dasJ removed their request for review November 21, 2022 12:50
@ofborg ofborg bot added 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. and removed 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. labels Nov 21, 2022
@ofborg ofborg bot requested a review from dasJ November 21, 2022 13:37
SuperSandro2000
SuperSandro2000 reviewed Nov 21, 2022
View reviewed changes
pkgs/development/libraries/openssl/default.nix
Comment on lines +197 to 199

${lib.optionalString (conf != null) "cat ${conf} > $etc/etc/ssl/openssl.cnf"}
'';
Copy link
Member

@SuperSandro2000 SuperSandro2000 Nov 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
${lib.optionalString (conf != null) "cat ${conf} > $etc/etc/ssl/openssl.cnf"}
'';
'' + lib.optionalString (conf != null) ''
cat ${conf} > $etc/etc/ssl/openssl.cnf
'';

Normally not a big fan of this but shouldn't this cut down the amount of rebuilds?

Copy link
Member Author

@ajs124 ajs124 Nov 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, but I'd guess not significantly, since this PR changes openldap and python3.

@mweinelt mweinelt merged commit 53d777c into NixOS:staging Nov 26, 2022
@ajs124 ajs124 deleted the init/openssl_legacy branch November 26, 2022 22:52
@github-actions github-actions bot mentioned this pull request Nov 26, 2022
Merged
1 task
@github-actions
Copy link
Contributor

github-actions bot commented Nov 26, 2022

Successfully created backport PR #203138 for staging-22.11.

@vcunat
Copy link
Member

vcunat commented Nov 30, 2022

This doesn't seem to work well on darwin, but I don't know why:
https://hydra.nixos.org/eval/1786377?filter=openldap#tabs-now-fail

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Nov 30, 2022

It doesn't like rc4

additional info: SASL(-1): generic failure: internal error: failed to init cipher 'rc4'

@vcunat
Copy link
Member

vcunat commented Nov 30, 2022 •
edited
Loading

Yes, I thought the need for RC4 was one of the reasons to try keeping older openssl in there. (and now a new reconfigured one)

@ajs124
Copy link
Member Author

ajs124 commented Nov 30, 2022

Hm. It works just fine on linux though. Maybe the openssl config file doesn't work the same way on darwin?

@vcunat
Copy link
Member

vcunat commented Dec 4, 2022

We'll need some way forward. This breaks really many builds (roughly 1000*2).

I could simply put the rm line back as optionalString stdenv.isDarwin, but perhaps it would be better if someone checked that we're not leaving some big problem in there.

@tjni
Copy link
Contributor

tjni commented Dec 5, 2022

I don't know why exactly, but #204554 seems to fix the issue.

@vcunat
Copy link
Member

vcunat commented Dec 5, 2022 •
edited
Loading

I'm not familiar with library loading on darwin.

But I believe that even even on Linux+glibc these library overrides are a bit difficult. I'm sure libldap.so is typically not used stand-alone, and some binaries using it probably use openssl already (not in _legacy variant). In those cases it's not clear which version is loaded first and thus wins. What I'm certain of is that when you run an executable (without using extra loading magic), a particular SONAME is satisfied just once, including in all libraries – and thus each symbol name is consistent across the whole executable.

This mechanism can also cause problems when impure loading happens, e.g. in particular with /run/opengl-driver.

EDIT: I don't know openldap well; maybe it will almost always get used as the executables (and thus have separate library loading) and not as libraries.

@ajs124
Copy link
Member Author

ajs124 commented Dec 5, 2022

Usage of openldap as an executable or library are both common

@vcunat
Copy link
Member

vcunat commented Dec 10, 2022

This is in nixos-unstable now, so I assume we're still backporting to 22.11 even though it's long past release?

@ajs124 ajs124 mentioned this pull request Aug 8, 2023
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@mweinelt mweinelt Awaiting requested review from mweinelt

@vcunat vcunat Awaiting requested review from vcunat

@FRidh FRidh Awaiting requested review from FRidh

@edolstra edolstra Awaiting requested review from edolstra

@np np Awaiting requested review from np

@adevress adevress Awaiting requested review from adevress

@dasJ dasJ Awaiting requested review from dasJ

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ajs124 @vcunat @mweinelt @SuperSandro2000 @tjni